Open
Bug 1422418
Opened 7 years ago
Updated 3 years ago
clang-format doesn't treat template functions return type the same as for functions
Categories
(Developer Infrastructure :: Lint and Formatting, enhancement)
Developer Infrastructure
Lint and Formatting
Tracking
(Not tracked)
NEW
People
(Reporter: glandium, Unassigned)
References
Details
Start with:
void foo(void) {
}
template<typename T>
void bar(T) {
}
clang-format reformats it as:
void
foo(void)
{
}
template<typename T>
void bar(T)
{
}
This might look trivial, but for more contrived examples, where this goes the line length limit, it changes things significantly:
verylongtype<with, template, arguments, all, the, way> long_function_name(void) {
}
template<typename Typename>
verylongtype<with, template, arguments, all, the, way> long_function_name(Typename) {
}
becomes:
verylongtype<with, template, arguments, all, the, way>
long_function_name(void)
{
}
template<typename Typename>
verylongtype<with, template, arguments, all, the, way> long_function_name(
Typename)
{
}
Comment 1•7 years ago
|
||
Good catch Mike, the issue is that when we pass to the function declaration types from the template argument list clang-format doesn't know to distinguish token long_function_name as a 'TT_FunctionDeclarationName' and it considers it to be 'TT_StartOfName' which is obviously wrong, since our logic to break before return type is like:
>> if (!Current->MustBreakBefore && InFunctionDecl &&
>> Current->is(TT_FunctionDeclarationName))
>> Current->MustBreakBefore = mustBreakForReturnType(Line);
I'm gonna look to see if we can change the way how we generate token: 'TT_FunctionDeclarationName'
Comment 2•7 years ago
|
||
One more thing that i can note here this is almost an honest mistake since in all our cases only type is present in the function declaration parameters, if we were to have something like:
>>template<typename Typename>
>>verylongtype<with, template, arguments, all, the, way> long_function_name(Typename var)
>>{
>>}
This would be formatted to:
>>template<typename Typename>
>>verylongtype<with, template, arguments, all, the, way>
>>long_function_name(Typename a)
>>{
>>}
This is due to the fact that clang-format uses a token stream for different token types recognition, and for our case there are several possibilities:
1. Only type is present so type must be part of the list of implicit types.
2. Type is also qualified so the token is deduced to be part of an argument list.
3. Token is not recognised so the starting token cannot be deduced as 'TT_FunctionDeclarationName'
Also we use a top level breaking point for the return type so I don't see the usage of constructions like this in top level places but only in nested levels:
>>template<typename Typename>
>>verylongtype<with, template, arguments, all, the, way> long_function_name(Typename) {
>>}
If I missed a place were we could use them please let me know.
Reporter | ||
Comment 3•7 years ago
|
||
(In reply to Andi-Bogdan Postelnicu [:andi] from comment #2)
> One more thing that i can note here this is almost an honest mistake since
> in all our cases only type is present in the function declaration
> parameters, if we were to have something like:
>
> >>template<typename Typename>
> >>verylongtype<with, template, arguments, all, the, way> long_function_name(Typename var)
> >>{
> >>}
>
> This would be formatted to:
>
> >>template<typename Typename>
> >>verylongtype<with, template, arguments, all, the, way>
> >>long_function_name(Typename a)
> >>{
> >>}
I was about to say that's not at all how that ends up formatted here, but in fact, I found another bug: .cc files are not treated the same as .cpp files. With the .cc extension, your example is not formatted like that, while it is with a .cpp extension.
Reporter | ||
Comment 4•7 years ago
|
||
Actually, .cc files are left untouched.
Comment 5•7 years ago
|
||
> Actually, .cc files are left untouched.
Yeah, we don't touch those extensions as, afaik, we don't have any in our source code (only in thirdparty)
https://dxr.mozilla.org/mozilla-central/source/tools/mach_commands.py#268
Reporter | ||
Comment 6•7 years ago
|
||
Filed bug 1423111.
Updated•7 years ago
|
Product: Core → Firefox Build System
Updated•7 years ago
|
Blocks: clang-format
Component: Source Code Analysis → Lint and Formatting
Updated•3 years ago
|
Product: Firefox Build System → Developer Infrastructure
Updated•3 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•